=spec #384 amend spec to allow not mentioning rule number in exception message #386
Conversation
…mber in exception message
README.md
Outdated
| @@ -183,7 +183,7 @@ public interface Subscription { | |||
| | [:bulb:](#3.7 "3.7 explained") | *The intent of this rule is superseded by [3.5](#3.5).* | | |||
| | <a name="3.8">8</a> | While the `Subscription` is not cancelled, `Subscription.request(long n)` MUST register the given number of additional elements to be produced to the respective subscriber. | | |||
| | [:bulb:](#3.8 "3.8 explained") | *The intent of this rule is to make sure that `request`-ing is an additive operation, as well as ensuring that a request for elements is delivered to the Publisher.* | | |||
| | <a name="3.9">9</a> | While the `Subscription` is not cancelled, `Subscription.request(long n)` MUST signal `onError` with a `java.lang.IllegalArgumentException` if the argument is <= 0. The cause message MUST include a reference to this rule and/or quote the full rule. | | |||
| | <a name="3.9">9</a> | While the `Subscription` is not cancelled, `Subscription.request(long n)` MUST signal `onError` with a `java.lang.IllegalArgumentException` if the argument is <= 0. The cause message SHOULD explain that non-positive request signals are illegal. | | |||
There was a problem hiding this comment.
There's an extraneous space between "cause" and "message".
I guess the question is that if amending this rule should actually be removing the last sentence, since the other rules which mandate exceptions are not requiring rule referencing. Thoughts @reactive-streams/contributors ?
There was a problem hiding this comment.
Ah, I see, then in the TCK we should not check the message at all if we want to remove the wording suggestion. It would only check that we got an IllegalArgumentException - which is also OK.
I don't mind either way, I like the suggestion but happy to remove and change the TCK PR to only check exception type.
There was a problem hiding this comment.
After some pondering I think just checking exception type is the most compatible thing to do anyway and to actually enforce - so I agree on removing the sentence completely.
Waiting a round of sleep for feedback and addressing those once i wake up :)
|
I think the current proposed wording is OK. |
|
Fixed the additional space and kept the wording, thanks for chiming in @DougLea ! |
|
@reactive-streams/contributors 1 day left to comment on this! |
|
@reactive-streams/contributors Merging. |
Resolves spec part of #384 and is follow up / prerequisite for the TCK change: #385
Discussion in: #345 (comment)
This change is compatible with existing implementations, and allows JDK9's interfaces to be compatible with our rules (since that impl won't refer to the number of the rule explicitly)
Resolves #385